-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Common power-libperfmgr bringup #1
base: fourteen-v3
Are you sure you want to change the base?
Conversation
* proprietary perfd blobs can finally be nuked without breaking goodix * we could even map the functions to use libperfmgr powerhints in the future [SebaUbuntu]: Cleanup Android.bp and add copyright header Change-Id: I124652f3041761966a3e3bd97c757fecc39cc5fb
It's pointless when using extern "C". Change-Id: Ibdf9f06a70aa3a75687b33781c78cf2172bb334d
Return a positive integer for perf lock acquire and release so that Goodix/FPC fingerprint sensor blobs will not complain. Goodix: E [goodixHAL][gf_hal_milan_f_series]: goodix_perf_lock_acquire: Failed to acquire perf lock, err: 0 E [goodixHAL][gf_hal_milan_f_series]: goodix_perf_lock_release: Perf lock release error 0 FPC: E fpc_tac : fpc_perf_lock_acquire: Incorrect params, Failed to acquire perf lock, err E fpc_tac : fpc_perf_lock_release: Perf lock release error 0 Signed-off-by: Chenyang Zhong <zhongcy95@gmail.com> Change-Id: I861672e9a738c2204755d802670f4b28b662f286
* Why bothering with C++ mangling when we can just build it as a standard C library? Change-Id: I45ea977edf7ea7fab6fece76f3049654a8d24c5d
This avoids conflicts with QCOM devices that are using the QTI perf implementation, which wouldn't want this library anyway as this one is a stub. Change-Id: I6258b7503bc45b8dced2022e6ff143fbe97e7c82
* Add and run clang-format * Add newlines between functions * Use elvis operator `?:` for return value Change-Id: I920044df12c99d32ddd2ef3fb5edf24bcd0c7360
WalkthroughThe recent changes introduce new files to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (4)
libqti-perfd-client/client.c (4)
12-12
: Placeholder function:perf_get_feedback
.The function is currently empty and might be a placeholder for future implementation.
Consider adding a comment to indicate that this function is intentionally left empty.
14-14
: Placeholder function:perf_hint
.The function is currently empty and might be a placeholder for future implementation.
Consider adding a comment to indicate that this function is intentionally left empty.
22-22
: Placeholder function:perf_lock_cmd
.The function is currently empty and might be a placeholder for future implementation.
Consider adding a comment to indicate that this function is intentionally left empty.
29-29
: Placeholder function:perf_lock_use_profile
.The function is currently empty and might be a placeholder for future implementation.
Consider adding a comment to indicate that this function is intentionally left empty.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- libqti-perfd-client/.clang-format (1 hunks)
- libqti-perfd-client/Android.bp (1 hunks)
- libqti-perfd-client/client.c (1 hunks)
Files skipped from review due to trivial changes (2)
- libqti-perfd-client/.clang-format
- libqti-perfd-client/Android.bp
Additional comments not posted (1)
libqti-perfd-client/client.c (1)
1-11
: LGTM! Header and includes are correct.The copyright notice and SPDX license identifier are appropriate. The included headers are necessary for logging and standard integer types.
int perf_lock_rel(int handle) { | ||
ALOGI("perf_lock_rel: handle: %d", handle); | ||
return handle ?: 233; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential portability issue with GCC extension.
The use of the ?:
ternary operator is a GCC extension and may not be portable. Consider using a standard ternary operator for better portability.
- return handle ?: 233;
+ return handle ? handle : 233;
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
int perf_lock_rel(int handle) { | |
ALOGI("perf_lock_rel: handle: %d", handle); | |
return handle ?: 233; | |
int perf_lock_rel(int handle) { | |
ALOGI("perf_lock_rel: handle: %d", handle); | |
return handle ? handle : 233; |
int perf_lock_acq(int handle, int duration, int arg3[], int arg4) { | ||
ALOGI("perf_lock_acq: handle: %d, duration: %d, arg3[0]: %d, arg4: %d", handle, duration, | ||
arg3[0], arg4); | ||
return handle ?: 233; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential portability issue with GCC extension.
The use of the ?:
ternary operator is a GCC extension and may not be portable. Consider using a standard ternary operator for better portability.
- return handle ?: 233;
+ return handle ? handle : 233;
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
int perf_lock_acq(int handle, int duration, int arg3[], int arg4) { | |
ALOGI("perf_lock_acq: handle: %d, duration: %d, arg3[0]: %d, arg4: %d", handle, duration, | |
arg3[0], arg4); | |
return handle ?: 233; | |
} | |
int perf_lock_acq(int handle, int duration, int arg3[], int arg4) { | |
ALOGI("perf_lock_acq: handle: %d, duration: %d, arg3[0]: %d, arg4: %d", handle, duration, | |
arg3[0], arg4); | |
return handle ? handle : 233; | |
} |
Summary by CodeRabbit
libqti-perfd-client
library within the Android ecosystem.These changes collectively enhance the maintainability and performance optimization of the project.